Fix ATmega32U4 _writeDutyCycle3PWM signature mismatch with hardware_api.h#553
Open
94xhn wants to merge 1 commit into
Open
Fix ATmega32U4 _writeDutyCycle3PWM signature mismatch with hardware_api.h#55394xhn wants to merge 1 commit into
94xhn wants to merge 1 commit into
Conversation
…pi.h The ATmega32U4-specific _writeDutyCycle3PWM had a stray unused `int pinA` parameter, giving it a 5-parameter signature that mismatches the 4-parameter prototype declared in hardware_api.h (and matched by all other 10 MCU implementations, plus the weak fallback in generic_mcu.cpp and the sole call site in BLDCDriver3PWM.cpp). Because C++ overload resolution treats differing parameter lists as different functions, this made the ATmega32U4-specific override permanently unreachable/dead code - any call to _writeDutyCycle3PWM on this MCU silently resolved to the generic weak fallback instead. The unused pinA parameter is removed so the function matches the declared interface and is restored as the actual override used by BLDCDriver3PWM for ATmega32U4 boards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
_writeDutyCycle3PWMinsrc/drivers/hardware_specific/atmega/atmega32u4_mcu.cpphad an extra, unusedint pinAparameter, giving it a 5-parameter signature:This mismatches the prototype declared in
src/drivers/hardware_api.h:...and the 4-parameter signature used by every other MCU implementation (atmega2560, atmega328, esp32 x2, renesas, rp2040, samd, silabs/efr32, stm32, teensy), the weak fallback in
generic_mcu.cpp, and the sole call site inBLDCDriver3PWM.cpp.Because differing parameter lists make these distinct overloads/symbols in C++, the ATmega32U4-specific
_writeDutyCycle3PWMcould never actually be called by anything in the codebase — it was permanently dead code, and any call fromBLDCDriver3PWMon an ATmega32U4 board silently resolved to the generic weak fallback instead of the MCU-specific override.pinAis never referenced anywhere in the 5-line function body (just threeanalogWritecalls), so removing it is a pure signature fix with no behavior change to the body itself — it just makes the function reachable again as the intended per-MCU override.Fix
Remove the unused
int pinAparameter so the function matcheshardware_api.hand the pattern used by every other MCU:Note on impact
Today the ATmega32U4-specific body and the generic weak-fallback body are functionally identical (both just do three
analogWritecalls with the same pin mapping), so there's no currently-observable runtime symptom from this bug on existing boards. This fix restores the correct per-MCU override/interface intent and removes unreachable/dead code — it also prevents a silent no-op trap for any future edit to the ATmega32U4-specific body (e.g. to actually use apinA-style parameter), which today would never take effect due to this signature mismatch.Testing
Verified by inspecting current
devbranch source directly (this repo has no CI build matrix I could run locally for AVR). Confirmed viagrepthat:hardware_api.h, the weak fallback ingeneric_mcu.cpp, all 10 other MCU implementations, and the single call site inBLDCDriver3PWM.cppall use the 4-parameter signature._writeDutyCycle3PWM.Also built an isolated minimal repro (separate from this repo) transplanting the exact buggy 5-param body and the exact 4-param call site, compiled with g++, and confirmed via
nmthat the 5-param version's mangled symbol never matches what the call site needs — while the fixed 4-param version's symbol matches exactly and links/runs correctly.